Skip to content

feat(persistent-worker): critic ping-pong protocol (#110)#140

Merged
hadamrd merged 1 commit into
trunkfrom
loop/110-feat-persistent-worker-critic-ping-pong
May 28, 2026
Merged

feat(persistent-worker): critic ping-pong protocol (#110)#140
hadamrd merged 1 commit into
trunkfrom
loop/110-feat-persistent-worker-critic-ping-pong

Conversation

@hadamrd
Copy link
Copy Markdown
Owner

@hadamrd hadamrd commented May 28, 2026

Closes #110.

Summary

  • Route a typed CriticReport through the AWAITING_CRITIC outbound edge.
  • approve → MERGED; request_changes → REVISING (+ counter bump + verbatim-prompt dispatch); block → ABANDONED + label loop:needs-review.
  • New format_critic_followup_prompt() serialises findings into the next worker prompt verbatim — every finding's message, file/line, and [sev/category] tag are preserved as the critic wrote them.
  • handle_critic_verdict() is the verdict→state router; it does NOT enforce the iteration cap (that's feat(persistent-worker): crash recovery — resume non-terminal sessions at boot #111).
  • Follow-up SDK session resumes via resume=<sdk_session_id> so the prompt cache stays warm.

Acceptance coverage

  • test_approve_transitions_to_merged — APPROVE → MERGED.
  • test_request_changes_transitions_to_revising_and_bumps_counter — REQUEST_CHANGES → REVISING, critic_iterations bumps via store.increment_iterations.
  • test_block_transitions_to_abandoned_and_labels_pr — BLOCK → ABANDONED + PR labelled loop:needs-review.
  • test_request_changes_dispatches_followup_with_verbatim_critic_comments — next dispatch prompt contains the critic comments verbatim.

Adversarial coverage

  • unknown verdict no-ops (next tick can retry the critic).
  • gh label failure does not undo the ABANDONED transition.
  • dispatch_revision crash does not undo REVISING / counter bump.
  • wrong starting state raises InvalidTransition.
  • unknown session id raises KeyError.
  • BLOCK without pr_url skips the label call.

Out of scope (separate tickets)

Test plan

  • pytest tests/test_critic_ping_pong.py (12 passed)
  • pytest tests/test_critic_iteration_cap.py tests/test_persistent_dispatch.py tests/test_critic.py (35 passed)
  • ruff check clean

🤖 Generated with Claude Code

Routes a typed CriticReport into the AWAITING_CRITIC outbound edge:

- approve         -> MERGED (auto-merge already in flight)
- request_changes -> REVISING (counter bumps via
                     store.increment_iterations, follow-up worker
                     dispatched with the SDK session_id resumed and
                     the critic comments serialised verbatim)
- block (sev1)    -> ABANDONED + PR labelled loop:needs-review

The verbatim contract — "no summarisation" — is delivered by
format_critic_followup_prompt(): every Finding's message, file/line,
and severity/category tag is rendered into the next prompt as-is, so
the resumed SDK worker sees exactly what the critic wrote.

dispatch_revision is a callback so unit tests can capture the prompt
and resume kwargs without actually spinning up an SDK session; the
runner wires the real dispatcher in a follow-up. A dispatch crash is
caught at the boundary so the FSM edge + counter bump are never
undone — the next tick can retry from REVISING.

Iteration-cap enforcement (#111) stays in enforce_critic_iteration_cap
and is layered on top by the runner; this handler is the pure
verdict→state router.

Tests cover the four acceptance cases plus adversarial paths:
unknown verdict no-ops, gh label failure can't undo ABANDONED,
dispatch crash can't undo REVISING, wrong starting state raises
InvalidTransition.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@hadamrd hadamrd merged commit 2f249c8 into trunk May 28, 2026
2 checks passed
@hadamrd
Copy link
Copy Markdown
Owner Author

hadamrd commented May 28, 2026

Source issue #110 was closed mid-flight (state: closed). Loop refusing auto-merge. Reopen the issue OR merge manually.

@hadamrd hadamrd deleted the loop/110-feat-persistent-worker-critic-ping-pong branch May 28, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(persistent-worker): critic ping-pong protocol

1 participant